[Python] Propagate WriteToFiles finalization failures - #39993
Conversation
|
Assigning reviewers: R: @jrmccluskey for label python. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
This is a significant performance concern we should revisit. It's a common use case FileIO processing millions of files and a known bottleneck to listing metadata from gcs files (#20137). I remember each GET call takes ~100 ms. This adds hours of wall time when write to large number of files |
|
If we want to be able to propagate error early, consider using Beam's dead letter queue framework that is allow user to choose an |
|
Thanks for contribution. We're trying to find ways handle increasing flux of contributions while keep the codebase robustness. As first step #40002 revised agent skills, PR labelling and review guidelines to advice both contributor and reviewer pay additional attention for core code paths. In the future the "Assigning reviewers" comment will contain the component specific guidelines for reviewers, e.g.
|
3 participants
WriteToFilescurrently catches everyBeamIOErrorfrom final renames and emits successfulFileResults even when the destination files are absent. A genuine I/O failure can therefore leave data in temporary files while the pipeline reports success.Propagate rename failures so the bundle can retry. Before renaming, skip a file only when its temporary source is absent and its final destination exists, preserving retries after complete or partial finalization. An existing destination does not suppress errors while the source remains.
Five regression tests cover failed renames without successful output, already-completed retries, partial moves followed by retry, stale destinations, and missing files. Against the unfixed implementation, four fail and the completed-retry compatibility test passes.
Testing
From
sdks/python, with the SDK and test dependencies installed:Python 3.12: 49 passed, 2 skipped. YAPF 0.43.0, Ruff 0.15.22, and
git diff --checkpass for the changed files.Downsides
Finalization adds one existence lookup per temporary file, plus a destination lookup when that source is absent. Remote filesystems incur these metadata requests before the existing batched rename. Genuine finalization errors now fail the bundle instead of being suppressed.
CHANGES.mdwith the behavior change.See the Contributor Guide and CI documentation.